Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Execute transaction through role #3768

Merged
merged 18 commits into from
Jun 5, 2024
Merged

feat: Execute transaction through role #3768

merged 18 commits into from
Jun 5, 2024

Conversation

jfschwarz
Copy link
Collaborator

@jfschwarz jfschwarz commented May 29, 2024

What it solves

With SEP-14 the community voted to implement role-based access control in Safe{Wallet} based on the Zodiac Roles Modifier.

How this PR fixes it

This PR allows role members to use Safe{Wallet} to execute transaction from the Safe using their role. For this purpose, a new section has been added to the transaction sign/execute form, which only shows up if the connected wallet is a member of any role enabled on the Safe.

How to test it

  • Deploy a Roles Modifier through the Zodiac Safe app
  • Set up some permissions for a testing role, e.g. using permissions-starter-kit, apply and assign the role to an EOA using the mod's assignRoles function

Alternatively, send me your EOA address so I can assign a role in my Sepolia test setup to it.

Screenshots

A new card appears in the transaction sign/execute form only if the Safe has a Roles mod enabled and the connected wallet account is a role member.

Connected with role member wallet, permission checks pass:
image
Connected with role member wallet, permission checks fail:
image

(Status can be any value from the Roles mods' Status enum)

Checklist

  • I've tested the branch on mobile 📱
  • I've documented how it affects the analytics (if at all) 📊
  • I've written a unit/e2e test for it (if applicable) 🧑‍💻

Implementation details

In any of the following cases, nothing will be rendered and no extra network requests will be made:

  • The transaction has already been pushed to the tx service and it in the signature collection phase
  • Safe is on a network without official Roles v2 support
  • There are no Safe modules enabled

Otherwise we perform a fetch request against the official role subgraph for each enabled module, checking if the module is a Roles v2 mod and retrieving its role configuration (permissions and members).

We only render the new card, if the connected wallet has any roles assigned. (There can be multiple Roles mods enabled on the Safe. An address can have multiple roles assigned on each mod.)

Next step is checking if the current transaction is allowed for any role assigned to the connected wallet address:

  • If the applicable permissions (matching target address and function selector) does not use a condition, we can statically determine if it allows the current transaction. No extra requests are required in this case.
  • If the applicable permission uses a condition we make an RPC call through the read-only provider for evaluating it on the given call.

If any assigned role allows the call, we enable the "Execute" button. Otherwise we show the failing permission check status for the role most likely to be meant for the given transaction, "most likely" being defined by the following order:

  • role allows target address and function selector
  • role allows target address, but not the function selector
  • role does't allow the target address

Copy link

github-actions bot commented May 29, 2024

Copy link

github-actions bot commented May 29, 2024

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type Occurrences Fixable
Errors 0 0
Warnings 0 0
Ignored 0 N/A
  • Result: ✅ success
  • Annotations: 0 total

Report generated by eslint-plus-action

Copy link

github-actions bot commented May 29, 2024

📦 Next.js Bundle Analysis for safe-wallet-web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

⚠️ Global Bundle Size Increased

Page Size (compressed)
global 950.36 KB (🟡 +295 B)
Details

The global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster.

Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis

If you want further insight into what is behind the changes, give @next/bundle-analyzer a try!

Six Pages Changed Size

The following pages changed size from the code in this PR compared to its base branch:

Page Size (compressed) First Load
/apps/open 50.93 KB (🟡 +33 B) 1001.29 KB
/transactions 73.93 KB (🟡 +33 B) 1 MB
/transactions/history 73.89 KB (🟡 +33 B) 1 MB
/transactions/messages 34.77 KB (🟡 +33 B) 985.13 KB
/transactions/queue 29.17 KB (🟡 +33 B) 979.53 KB
/transactions/tx 18.92 KB (🟡 +33 B) 969.28 KB
Details

Only the gzipped size is provided here based on an expert tip.

First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If next/link is used, subsequent page loads would only need to download that page's bundle (the number in the "Size" column), since the global bundle has already been downloaded.

Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis

Next to the size is how much the size has increased or decreased compared with the base branch of this PR. If this percentage has increased by 20% or more, there will be a red status indicator applied, indicating that special attention should be given to this.

Copy link

github-actions bot commented May 29, 2024

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
79.41% (-0.04% 🔻)
11468/14442
🔴 Branches
58.51% (+0.1% 🔼)
2758/4714
🟡 Functions
66.63% (+0.07% 🔼)
1843/2766
🟢 Lines
80.78% (-0.06% 🔻)
10349/12812
Show new covered files 🐣
St.
File Statements Branches Functions Lines
🟢
... / index.tsx
89.29% 79.17% 100% 89.02%
🟢
... / hooks.ts
79.44% 61.82% 81.82% 80%
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🔴
... / dispatch.ts
38.54% (-3.08% 🔻)
34.48% (-5.52% 🔻)
26.09% (-3.91% 🔻)
37.43% (-3.56% 🔻)
🔴
... / index.ts
43.48% (-23.19% 🔻)
0%
16.67% (-8.33% 🔻)
41.18% (-21.32% 🔻)

Test suite run success

1441 tests passing in 198 suites.

Report generated by 🧪jest coverage report action from 822271d

@TanyaEfremova
Copy link
Contributor

Hey @jfschwarz, great addition! A few suggestions to improve the texts:

Execute without confirmations
As a member of the eth_wrapping role you can execute this transaction immediately without confirmations from other owners.
Button label: Execute

Copy link

github-actions bot commented May 31, 2024

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type Occurrences Fixable
Errors 0 0
Warnings 0 0
Ignored 0 N/A
  • Result: ✅ success
  • Annotations: 0 total

Report generated by eslint-plus-action

Copy link

github-actions bot commented Jun 5, 2024

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type Occurrences Fixable
Errors 0 0
Warnings 0 0
Ignored 0 N/A
  • Result: ✅ success
  • Annotations: 0 total

Report generated by eslint-plus-action

@jfschwarz jfschwarz force-pushed the exec-through-role branch from 8592dc1 to fd6d915 Compare June 5, 2024 10:51
Copy link
Member

@katspaugh katspaugh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @jfschwarz! Excellent work!

@katspaugh katspaugh merged commit 2ecdfef into dev Jun 5, 2024
13 of 14 checks passed
@katspaugh katspaugh deleted the exec-through-role branch June 5, 2024 12:10
Copy link

gitpoap-bot bot commented Jun 5, 2024

Congrats, your important contribution to this open-source project has earned you a GitPOAP!

GitPOAP: 2024 Safe Contributor:

GitPOAP: 2024 Safe Contributor GitPOAP Badge

Head to gitpoap.io & connect your GitHub account to mint!

Learn more about GitPOAPs here.

@github-actions github-actions bot locked and limited conversation to collaborators Jun 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants